USE 557 - identify records for harvest and parse bitstream download information#9
Conversation
…ation Why these changes are being introduced: This harvester is designed to retrieve fulltext for DSpace theses in TIMDEX with the goal of ultimately writing it back to the TIMDEX dataset associated with particular records. As such, we can read records from the TIMDEX dataset to use as the driver of what records this harvester will harvest for. Additionally, the original DSpace METS is present in the TIDMEX dataset and provides information about DSpace item bitstreams, including the TEXT bitstream we aim to harvest. How this addresses that need: A new module `dfh.timdex_dataset` is created with a class `TIMDEXThesesRecords` that is designed to find DSpace theses records in the TIMDEX dataset, optionally filtered by `run_id`, and parse information from those record's source DSpace METS about bitstreams. The final result is an iterator of TIMDEX records + bitstream information we can use to harvest the fulltext from DSpace. The objects will be used by this harvester to a) harvest fulltext from DSpace and b) write the fulltext back to the TIMDEX dataset associated with the right record. Side effects of this change: * None really, nothing yet depends on this. Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/USE-557
Why these changes are being introduced: There is one primary interaction with the DSpace API: generating pre-signed S3 URLs of bitstreams to download. To do so, we'll need to authenticate and send GET and/or POST requests. How this addresses that need: Instead of importing a fully featured DSpace 8 / CRIS client, a slimmed down version is created here that does three things well: 1. authenticates 2. supports generic, authenticated GET/POST requests 3. opinionation for the one currently known API request we'll make, generating pre-signed URLs Side effects of this change: * This departs from other applications like DSC and DSS which use a community DSpace 8 / CRIS API library. It would be very straight-forward to pivot to using that in the future for this application if needed, but it seems heavier than what we need at the moment. Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/USE-557
| self._update_xsrf_token(r) | ||
| return r | ||
|
|
||
| def get_presigned_url_for_bitstream(self, bitstream_uuid: str) -> str: |
There was a problem hiding this comment.
Convenience method for a pre-signed URL, leaning on a self.api_get() call.
| def _init_timdex_dataset(self, dataset_location: str | None) -> TIMDEXDataset: | ||
| """Init TIMDEXDataset and compose to self. | ||
|
|
||
| If dataset_location is not passed, look to env var TIMDEX_DATASET_LOCATION. |
There was a problem hiding this comment.
This is the first TIMDEX application that optionally uses a TIMDEX_DATASET_LOCATION env var. The CLI will likely still require it get passed, to align with other TIMDEX apps in the pipeline... but looking to establish this alternate pattern here and now with the hopes perhaps we can move to an env var for most/all apps at some point.
| class TIMDEXThesesRecords: | ||
| """Class to retrieve TIMDEX dataset records of DSpace Theses. | ||
|
|
||
| Theses are determined by looking for "Thesis" in transformed_record.content_type. | ||
| """ |
There was a problem hiding this comment.
In the event we broaden our harvesting of fulltext beyond theses, it would not be difficult to add some inheritance here:
TIMDEXRecordsTIMDEXThesesRecords(TIMDEXRecords)- all shared except opionated filtering to theses records only
I mention this only to point out that while we have some theses-opinionation in the business logic here, it will be easy to extend if and when needed. Opted to focus on the immediate need.
There was a problem hiding this comment.
Pull request overview
Introduces the initial harvesting “selection + lookup” layer by (1) iterating TIMDEX DSpace thesis records, (2) extracting TEXT bitstream metadata from embedded DSpace METS, and (3) providing a lightweight DSpace REST client to generate pre-signed download URLs for known bitstream UUIDs.
Changes:
- Add
TIMDEXThesesRecordsto fetch/filter DSpace thesis records from the TIMDEX dataset and parse TEXT bitstream metadata from METS. - Add
DSpaceClientto authenticate against DSpace and request signed URLs for bitstreams. - Add pytest coverage and fixtures for METS parsing edge-cases; adjust lint/typechecker configuration.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
dfh/timdex_dataset.py |
New TIMDEX dataset helper for filtering thesis records and extracting TEXT bitstream info from METS XML. |
dfh/dspace.py |
New lightweight DSpace REST client for auth + signed URL retrieval. |
tests/test_dataset.py |
New tests covering TIMDEXThesesRecords iteration and METS parsing error cases. |
tests/conftest.py |
Adds fixtures for TIMDEX record JSON + various METS XML inputs. |
tests/fixtures/timdex/dspace-1721.1-139336.json |
TIMDEX thesis record fixture used by dataset tests. |
tests/fixtures/dspace/mets/1721.1-139336.xml |
“Happy path” METS fixture with TEXT fileGrp. |
tests/fixtures/dspace/mets/1721.1-139336-missing-text-filegrp.xml |
METS fixture missing TEXT fileGrp (error case). |
tests/fixtures/dspace/mets/1721.1-139336-text-filegrp-missing-file.xml |
METS fixture with empty TEXT fileGrp (error case). |
tests/fixtures/dspace/mets/1721.1-139336-text-file-missing-flocat.xml |
METS fixture missing FLocat for TEXT file (error case). |
tests/fixtures/dspace/mets/1721.1-139336-text-flocat-missing-href.xml |
METS fixture missing href in FLocat (error case). |
pyproject.toml |
Updates mypy excludes/overrides and ruff configuration (exclusions/ignore list). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """ | ||
|
|
||
|
|
||
| class TextBitstreamInfo(TypedDict): |
There was a problem hiding this comment.
Was there a reason to not use FulltextBitstreamInfo? I see you've bounced between text and fulltext in the names, comments, and docstrings
There was a problem hiding this comment.
Nice catch - no good reason. I'll try and normalize on "Fulltext".
There was a problem hiding this comment.
You know, on second thought, maybe there was rhyme and reason.
We know that we're going after "fulltext" for theses. But technically the bitstream is TEXT.
If you're okay with it @ehanson8, I'd kind of like to keep as-is. Tried to use "fulltext" in places where we're speaking about it conceptually, but often "text" in classes like this and methods that are quite literally interacting with the TEXT bitstream.
There was a problem hiding this comment.
That makes sense, a docstring clarifying this could be helpful but not required
Why these changes are being introduced: There is a relatively new DSpace 8 / CRIS client `dspace-rest-client`, that we are using in other projects. We would benefit from normalizing to use that here as well. Even though are needs are lightweight here, it handles the authentication well and provides an identical surface to work from (the "lightweight" client here in code was cribbed heavily from it to begin with). How this addresses that need: * Removes custom `DSpaceClient` class * Replaces with two functions: one to get a `DSpaceClient` instance and a second to generate a pre-signed bitstream download URL using that client Side effects of this change: * Take on dependencies of the 3rd party client `dspace-rest-python`, but also benefit from the community improvements to auth, dependency updates, etc. Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/USE-557
Purpose and background context
This PR introduces the mechanism by which this application will identify records to harvest fulltext for. And, a lightweight DSpace client to generate pre-signed URLs for known bitstream UUIDs.
For now, this application only wants to harvest fulltext bitstreams for DSpace theses. Ultimately, it will want to write the fulltext back to the TIMDEX dataset under a new data type called "fulltexts" (see USE-531), associated with specific TIMDEX records from the "records" data type. As such, it makes sense to read records from the TIMDEX dataset as the primary driver for what to harvest.
Additionally, the DSpace theses records in the TIMDEX dataset have their original DSpace METS which conveniently includes bitstream information! By utilizing that, we can avoid 3-4 DSpace APIs calls per item, and skip directly to generating a pre-signed S3 URL for a known bitstream UUID as the only DSpace API call we'll need to make.
Functionality added:
Functionality coming based on this:
How can a reviewer manually see the effects of these changes?
1- Test new class
TIMDEXThesesRecordsSet Dev1
TimdexManagerscredentials.Create
.envfile:Start ipython shell:
Init
TIMDEXThesesRecordsinstance, limited to 10 records:Note that no
run_idfiltering is applied here, thereby defaulting to all, currentdspacerecords that are deemed to be theses.Retrieve all records as a list from the iterator:
Observe first record:
Note: ignore the testing DSpace host found in the record.
With this in hand from this class + iterator, we have enough information to:
b0e3c854-de3e-44ce-94a3-6088615a5eeb(timdex_record_id="dspace:1721.1-150510", run_id="5e11f27e-fe05-44f4-8bcb-5acbdb032cf3", run_record_offset="100001")This iterator is quite quick. The I/O bottleneck will be in future work of parallelizing the requests for pre-signed bitstream URLs from this information.
Note
This section has been updated based on our pivot to using
dspace-rest-pythonclient versus a lightweight, custom client for DSpace API interactions.2- Test generation of pre-signed URLs for bitstream downloading
Add the following to your
.envfile and restart ipython shell, with the values shared outside of this PR:NOTE: at this time, this application is expecting these env vars to authenticate if API base + username + password is not explicitly passed during
DSpaceClientinit. Still in flux how this app will receive the credentials. It's possible they will be parsed from another secret, and likely passed to the client init. The client itself will likely remain defaulting to these three env vars for ease of testing.Get a
DSpaceClientinstance::Note that authentication is automatic due to
auth_on_initdefaulting toTruein function.Generate a pre-signed S3 URL for the bitstream shown above:
The link string that is returned is only valid for a single use and for a limited amount of time; both of which are easily satisfied by this harvester as it loops through records and utilizes the URLs.
Includes new or updated dependencies?
YES
Changes expectations for external applications?
NO
What are the relevant tickets?
Code review